-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Fix image drop from main editor #7910
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix image drop from main editor #7910
Conversation
…-image-drop-from-main-editor
…-image-drop-from-main-editor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
10 issues found across 9 files
Prompt for AI agents (all 10 issues)
Understand the root cause of the following 10 issues and fix them.
<file name="extensions/vscode/src/extension/VsCodeMessenger.ts">
<violation number="1" location="extensions/vscode/src/extension/VsCodeMessenger.ts:295">
Non-PNG images are all forced to image/jpeg; gif/webp/svg (and others) will be mislabeled, causing incorrect data URLs.</violation>
<violation number="2" location="extensions/vscode/src/extension/VsCodeMessenger.ts:295">
Extension comparison is case-sensitive; uppercase extensions (e.g., .PNG) are misclassified, producing the wrong MIME type.</violation>
</file>
<file name="gui/src/components/mainInput/TipTapEditor/TipTapEditor.tsx">
<violation number="1" location="gui/src/components/mainInput/TipTapEditor/TipTapEditor.tsx:226">
Unconditional hide overrides delayed/conditional logic in onDragLeave, making the delay and Shift-key behavior ineffective.</violation>
<violation number="2" location="gui/src/components/mainInput/TipTapEditor/TipTapEditor.tsx:241">
onDrop does not prevent default, which can cause the browser to navigate/open the dropped file when dropping outside the editor area. Add event.preventDefault() to avoid data loss risk.</violation>
</file>
<file name="gui/src/components/mainInput/TipTapEditor/utils/editorConfig.ts">
<violation number="1" location="gui/src/components/mainInput/TipTapEditor/utils/editorConfig.ts:151">
Drop handler prevents default and returns true even when nothing is handled, likely blocking normal text/URL drops. Consider only preventing default/returning true when an image is actually processed, otherwise fall through.</violation>
<violation number="2" location="gui/src/components/mainInput/TipTapEditor/utils/editorConfig.ts:189">
Image from HTML drop is inserted at position 0 instead of the intended drop/caret position.</violation>
</file>
<file name="gui/src/components/mainInput/TipTapEditor/utils/imageUtils.ts">
<violation number="1" location="gui/src/components/mainInput/TipTapEditor/utils/imageUtils.ts:44">
Avoid logging absolute local file paths to prevent leaking sensitive user information.</violation>
<violation number="2" location="gui/src/components/mainInput/TipTapEditor/utils/imageUtils.ts:66">
Avoid logging the full response object; it may contain large base64 data and sensitive content. Log only metadata (e.g., type or status) instead.</violation>
<violation number="3" location="gui/src/components/mainInput/TipTapEditor/utils/imageUtils.ts:146">
Logging the entire HTML content can expose sensitive data and add overhead. Prefer logging a short, non-sensitive summary.</violation>
<violation number="4" location="gui/src/components/mainInput/TipTapEditor/utils/imageUtils.ts:154">
Do not log full resource URLs that may include local file paths; log a generic message instead.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai
to give feedback, ask questions, or re-run the review.
const fileUri = vscode.Uri.file(filepath); | ||
const fileContents = await vscode.workspace.fs.readFile(fileUri); | ||
const fileType = | ||
filepath.split(".").pop() === "png" ? "image/png" : "image/jpeg"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-PNG images are all forced to image/jpeg; gif/webp/svg (and others) will be mislabeled, causing incorrect data URLs.
Prompt for AI agents
Address the following comment on extensions/vscode/src/extension/VsCodeMessenger.ts at line 295:
<comment>Non-PNG images are all forced to image/jpeg; gif/webp/svg (and others) will be mislabeled, causing incorrect data URLs.</comment>
<file context>
@@ -287,6 +287,18 @@ export class VsCodeMessenger {
+ const fileUri = vscode.Uri.file(filepath);
+ const fileContents = await vscode.workspace.fs.readFile(fileUri);
+ const fileType =
+ filepath.split(".").pop() === "png" ? "image/png" : "image/jpeg";
+ const dataUrl = `data:${fileType};base64,${Buffer.from(
+ fileContents,
</file context>
const fileUri = vscode.Uri.file(filepath); | ||
const fileContents = await vscode.workspace.fs.readFile(fileUri); | ||
const fileType = | ||
filepath.split(".").pop() === "png" ? "image/png" : "image/jpeg"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extension comparison is case-sensitive; uppercase extensions (e.g., .PNG) are misclassified, producing the wrong MIME type.
Prompt for AI agents
Address the following comment on extensions/vscode/src/extension/VsCodeMessenger.ts at line 295:
<comment>Extension comparison is case-sensitive; uppercase extensions (e.g., .PNG) are misclassified, producing the wrong MIME type.</comment>
<file context>
@@ -287,6 +287,18 @@ export class VsCodeMessenger {
+ const fileUri = vscode.Uri.file(filepath);
+ const fileContents = await vscode.workspace.fs.readFile(fileUri);
+ const fileType =
+ filepath.split(".").pop() === "png" ? "image/png" : "image/jpeg";
+ const dataUrl = `data:${fileType};base64,${Buffer.from(
+ fileContents,
</file context>
filepath.split(".").pop() === "png" ? "image/png" : "image/jpeg"; | |
(filepath.split(".").pop() || "").toLowerCase() === "png" ? "image/png" : "image/jpeg"; |
} | ||
}); | ||
event.preventDefault(); | ||
// Let the event bubble to ProseMirror by not preventing default |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onDrop does not prevent default, which can cause the browser to navigate/open the dropped file when dropping outside the editor area. Add event.preventDefault() to avoid data loss risk.
Prompt for AI agents
Address the following comment on gui/src/components/mainInput/TipTapEditor/TipTapEditor.tsx at line 241:
<comment>onDrop does not prevent default, which can cause the browser to navigate/open the dropped file when dropping outside the editor area. Add event.preventDefault() to avoid data loss risk.</comment>
<file context>
@@ -221,40 +222,23 @@ function TipTapEditorInner(props: TipTapEditorProps) {
- }
- });
- event.preventDefault();
+ // Let the event bubble to ProseMirror by not preventing default
}}
>
</file context>
// Let the event bubble to ProseMirror by not preventing default | |
event.preventDefault(); |
} else { | ||
setTimeout(() => setShowDragOverMsg(false), 2000); | ||
setTimeout(() => { | ||
setShowDragOverMsg(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unconditional hide overrides delayed/conditional logic in onDragLeave, making the delay and Shift-key behavior ineffective.
Prompt for AI agents
Address the following comment on gui/src/components/mainInput/TipTapEditor/TipTapEditor.tsx at line 226:
<comment>Unconditional hide overrides delayed/conditional logic in onDragLeave, making the delay and Shift-key behavior ineffective.</comment>
<file context>
@@ -221,40 +222,23 @@ function TipTapEditorInner(props: TipTapEditorProps) {
} else {
- setTimeout(() => setShowDragOverMsg(false), 2000);
+ setTimeout(() => {
+ setShowDragOverMsg(false);
+ }, 2000);
}
</file context>
const plugin = new Plugin({ | ||
props: { | ||
handleDOMEvents: { | ||
drop(view, event) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drop handler prevents default and returns true even when nothing is handled, likely blocking normal text/URL drops. Consider only preventing default/returning true when an image is actually processed, otherwise fall through.
Prompt for AI agents
Address the following comment on gui/src/components/mainInput/TipTapEditor/utils/editorConfig.ts at line 151:
<comment>Drop handler prevents default and returns true even when nothing is handled, likely blocking normal text/URL drops. Consider only preventing default/returning true when an image is actually processed, otherwise fall through.</comment>
<file context>
@@ -147,6 +148,80 @@ export function createEditorConfig(options: {
const plugin = new Plugin({
props: {
handleDOMEvents: {
+ drop(view, event) {
+ // Hide drag overlay immediately when drop is handled
+ setShowDragOverMsg(false);
</file context>
const node = schema.nodes.image.create({ | ||
src: dataUrl, | ||
}); | ||
const tr = view.state.tr.insert(0, node); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Image from HTML drop is inserted at position 0 instead of the intended drop/caret position.
Prompt for AI agents
Address the following comment on gui/src/components/mainInput/TipTapEditor/utils/editorConfig.ts at line 189:
<comment>Image from HTML drop is inserted at position 0 instead of the intended drop/caret position.</comment>
<file context>
@@ -147,6 +148,80 @@ export function createEditorConfig(options: {
+ const node = schema.nodes.image.create({
+ src: dataUrl,
+ });
+ const tr = view.state.tr.insert(0, node);
+ view.dispatch(tr);
+ }
</file context>
@aadarshkt it doesn't appear that I have access to push to this PR, could you either change this or run the following and add a commit? git checkout main -- .vscode/launch.json
git checkout main -- .vscode/tasks.json |
Description
#7908
Reopening this with requested changes: